-
-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: delete_user() should remove both grouping policies and policies #292
Conversation
@smrpn @hackerchai @PsiACE @GopherJ please review |
Codecov Report
@@ Coverage Diff @@
## master #292 +/- ##
==========================================
- Coverage 81.03% 81.02% -0.01%
==========================================
Files 23 23
Lines 3337 3342 +5
==========================================
+ Hits 2704 2708 +4
- Misses 633 634 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi @PsiACE , thanks for reviewing! I'm a GSoC 2022 candidate applying for Rust project. I've contributed to several Casbin repos like casdoor, casnode, and casbin-website, and now I want to make some contributions to |
It is good to see that you have chosen Casbin Rust in GSoC 2022. At this stage, we encourage students to get involved in the maintenance of Casbin Rust and its surrounding ecology. If you have any questions, you can also contact me by email. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
For now,
delete_user()
only removes grouping policies:casbin-rs/src/rbac_api.rs
Lines 259 to 262 in ef5ef67
But other versions (Golang, Node, Python, etc.) remove both grouping policies and policies, take Golang version for example:
https://github.com/casbin/casbin/blob/cb944fd5ef0a0029b6e74b8ca658f7dcb72e69f9/rbac_api.go#L95-L104
Previous discussion is here ( casbin/node-casbin#118 ) , and participants including @hsluoyz agreed that
delete_user()
should also delete policies, not only grouping policies.Related PRs:
PS: I'm a GSoC 2022 candidate and submitted my proposal for
casbin-rs
. I've made several PRs to Casbin community, this is my first PR tocasbin-rs
, glad to meet all of you!